-
-
Notifications
You must be signed in to change notification settings - Fork 299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[11.0][MIG] : mig account_invoice_import #41
Conversation
92d9533
to
3c1afd4
Compare
Coverage remained the same at 41.483% when pulling 3c1afd4e57d034d1288cca189344edc0be412f8a on njeudy:11-mig-account_invoice_import into b13c8e9 on OCA:11.0. |
3c1afd4
to
1721baf
Compare
<field name="model">res.partner</field> | ||
<field name="inherit_id" ref="account.view_partner_property_form"/> | ||
<field name="arch" type="xml"> | ||
<field name="currency_id" position="after"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I would prefer:
<xpath expr="//group[@name='acc_purchase']" position="inside">
<field name="arch" type="xml"> | ||
<xpath expr="//h2[contains(text(),'Taxes')]" position="before"> | ||
<h2 attrs="{'invisible': [('has_chart_of_accounts','=',False)]}">Invoice Import</h2> | ||
<div class="row mt16 o_settings_container" attrs="{'invisible': [('has_chart_of_accounts','=',False)]}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you indent a bit less?
account_invoice_import/README.rst
Outdated
@@ -0,0 +1,91 @@ | |||
.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use the latest template? The .png image is preferable.
2e49e72
to
87a5dd7
Compare
f.seek(0) | ||
invoice = f.read() | ||
f.close() | ||
inv_b64 = invoice.encode('base64') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use base64.b64encode(invoice)
self.ensure_one() | ||
assert self.invoice_file, 'No invoice file' | ||
logger.info('Starting to import invoice %s', self.invoice_filename) | ||
file_data = self.invoice_file.decode('base64') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use base64.b64decode(self.invoice_file)
fe7ea04
to
9c87252
Compare
@njeudy you may want to approve this easy one OCA/community-data-files#34, to speed up the pending PRs that are required. |
f0fa9ff
to
bad58b3
Compare
@alexis-via @astirpe coverage decrease with this PR, can you help me to improve test coverage ? Need I add real pdf invoice to import and try différent config on it ? |
new_res = [] | ||
for (inv_id, name) in res: | ||
inv = self.browse(inv_id) | ||
display_currency = inv.currency_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add line
pre = post = u''
to avoid this error:
Error:
Odoo Server Error
Traceback (most recent call last):
File "/opt/odoo/odoo-11.0/odoo/http.py", line 647, in _handle_exception
return super(JsonRequest, self)._handle_exception(exception)
File "/opt/odoo/odoo-11.0/odoo/http.py", line 307, in _handle_exception
raise pycompat.reraise(type(exception), exception, sys.exc_info()[2])
File "/opt/odoo/odoo-11.0/odoo/tools/pycompat.py", line 87, in reraise
raise value
File "/opt/odoo/odoo-11.0/odoo/http.py", line 689, in dispatch
result = self._call_function(**self.params)
File "/opt/odoo/odoo-11.0/odoo/http.py", line 339, in _call_function
return checked_call(self.db, *args, **kwargs)
File "/opt/odoo/odoo-11.0/odoo/service/model.py", line 97, in wrapper
return f(dbname, *args, **kwargs)
File "/opt/odoo/odoo-11.0/odoo/http.py", line 332, in checked_call
result = self.endpoint(*a, **kw)
File "/opt/odoo/odoo-11.0/odoo/http.py", line 933, in __call__
return self.method(*args, **kw)
File "/opt/odoo/odoo-11.0/odoo/http.py", line 512, in response_wrap
response = f(*args, **kw)
File "/opt/odoo/odoo-server/addons/web/controllers/main.py", line 930, in call_kw
return self._call_kw(model, method, args, kwargs)
File "/opt/odoo/odoo-server/addons/web/controllers/main.py", line 922, in _call_kw
return call_kw(request.env[model], method, args, kwargs)
File "/opt/odoo/odoo-11.0/odoo/api.py", line 689, in call_kw
return call_kw_multi(method, model, args, kwargs)
File "/opt/odoo/odoo-11.0/odoo/api.py", line 680, in call_kw_multi
result = method(recs, *args, **kwargs)
File "/opt/odoo/custom/addons/account_invoice_import/models/account_invoice.py", line 26, in name_get
inv.amount_untaxed, pre=pre, post=post)
UnboundLocalError: local variable 'post' referenced before assignment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python 3 every string is unicode, so no need for the u
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok will do this. @astirpe any idea to improve coverage ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I cannot check the coverage, as one test is failing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I create a merge request for failing test because it is not on this module
here it is : #53
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line you added pre = post = ''
is in the wrong place. It should be placed inside the for loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Now the other line, the one outside the loop (pre = post = ''
), can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean line 13
e0dacbf
to
94bf720
Compare
@pedrobaeza is it possible in oca dependency to import other branch of same repo to have travis green ? |
Uhm, I don't think so... Try to put on oca_dependencies.txt the name of the organization and the branch to see it if works. Any way, better to work on merging the other PR, so we don't need this hack. |
Ok will remove hack and work on merging PR thanks |
Scenario:
I get this Server Error - Warning:
The error above is raised because the vendor bill does not contain any tax line yet, as it was created empty. As a workaround, I did this:
The product line and the tax line are correctly imported. My request is: is it possible to automatically import the tax lines without that workaround? The calculation of the tax line can be based on the just imported product lines and the fiscal position of the vendor (as usual). |
@astirpe i will check this .. can you reproduce this on V10 ? (V10 is the standard up to date branch for this module I think .. ) |
@njeudy I guess is the same in V10, since this part of code is the same. In this case we could open a separate issue if you prefer. |
parsed_inv['amount_untaxed'] | ||
invoice.tax_line_ids[0].amount = tax_amount | ||
cur_symbol = invoice.currency_id.symbol | ||
invoice.message_post( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make the message translatable, as it was done in V10? (the fix of PR #51 should be ported to V11)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
94bf720
to
e20c44c
Compare
One of the zugferd-specific code has been standardised in UNECE, so moved to the account_tax_unece module
Fix update invoice via import wizard
… better support out customer invoice import Fix test suite of account_invoice_import (Agrolait is only a customer in v10)
* Update to work with latest version of invoice2data * Add requirements.txt file
…not deduct VAT (OCA#64) * Move code to a new dedicated method * FIX update of existing draft invoice Improve error message of partner matching Move hook of partner matching to put it after VAT matching, which is the most reliable * Add support for importing an invoice with VAT in a company that cannot deduc VAT Display nice error when account is missing on product * Improve compliancy with Factur-X standard (using schematron)
… has visible discounts Code improvements in sale_order_import Add unit tests in sale_order_import Use display_name instead of name_get()[0][1]
Update other modules accordingly
Code update and cleanup
Update the account_invoice_download_weboob following the changes in account_invoice_download Add README.rst for the 2 modules
Add weboob module management Improve log handling (explicit error when wrong pwd) account_invoice_import: by default, set tax to default tax of account when invoice_line_method = 1line_no_product
Update text displayed in the invoice import wizard and make list of supported formats modular (like in bank statement import)
… in multi-company environnement Fix my previous commit on sequence field on invoice import config (caused a crash when creating a new record) Usability improvements on download config
…iness document is imported in the right company
d29c854
to
957392f
Compare
@pedrobaeza squashed, the commits are reduced from 115 to 83. Would you give it a check? |
@pedrobaeza I'm tempted to merge this one. Is it ok? |
OK, we can reduce a bit more the history on 12.0. Thanks all for the efforts! |
PR needed: